Skip to content

Conversation

@arpitbbhayani
Copy link

The AOF rewrite tests were failing because bloom filter data was not being properly restored after server restart. The root cause was twofold:

  1. BF.LOAD command was replicating during AOF loading, causing duplicate entries
  2. Tests were not properly configuring AOF persistence for server restart

Changes:

  • Modified BF.LOAD handler to skip replication when loading from AOF by checking must_obey_client() context
  • Updated AOF tests to enable appendonly before adding data
  • Added wait for initial AOF rewrite to prevent concurrent rewrites
  • Set appendonly in server startup args for proper AOF loading on restart

Also includes minor fixes:

  • Changed build.sh shebang to bash for better compatibility
  • Updated pytest requirement to 7.4.3
  • Fixed indentation in utils.rs documentation

Fixes #74

The AOF rewrite tests were failing because bloom filter data was not
being properly restored after server restart. The root cause was
twofold:

1. BF.LOAD command was replicating during AOF loading, causing
   duplicate entries
2. Tests were not properly configuring AOF persistence for server
   restart

Changes:
- Modified BF.LOAD handler to skip replication when loading from AOF
  by checking must_obey_client() context
- Updated AOF tests to enable appendonly before adding data
- Added wait for initial AOF rewrite to prevent concurrent rewrites
- Set appendonly in server startup args for proper AOF loading on
  restart

Also includes minor fixes:
- Changed build.sh shebang to bash for better compatibility
- Updated pytest requirement to 7.4.3
- Fixed indentation in utils.rs documentation

Fixes valkey-io#74
// if filter not exists, create it.
let hex = value.to_vec();
let validate_size_limit = !must_obey_client(ctx);
let is_aof_loading = must_obey_client(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this intermediary step here of having the variable of is_aof_loading could we not use validate_size_limit on line 829?

@enjoy-binbin
Copy link
Member

@arpitbbhayani can you fix the DCO?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants